v6.10.0 proposal - #9711
Conversation
* feat(llmobs): accept image_parts on messages
Adds image support to the LLM Observability SDK, mirroring audio_parts. A
message may carry imageParts, each `{mimeType, content | attachmentKey}`, which
the tagger validates and emits as the snake_case wire shape `image_parts:
[{mime_type, content | attachment_key}]` — the same shape dd-trace-py emits and
the backend already types.
formatAudioPart and formatImagePart share one builder, since audio and image
parts have an identical wire shape and the linter rejects the duplicate.
Manual annotation only; provider auto-capture is a follow-up.
* feat(llmobs): mirror image part types to v5 and tighten ImagePart
Address review feedback on the public typing surface.
index.d.v5.ts now declares Message.imageParts and ImagePart. AGENTS.md
requires a new public type in both files unless the API is v6-only, and
this one is not: the runtime backports and audioParts already ships in
v5. No tsconfig references index.d.v5.ts, so it was verified by compiling
that surface standalone and resolving llmobs.ImagePart against it.
ImagePart becomes an exclusive union carrying exactly one of content or
attachmentKey, using the "?: never" shape already used by
AssistantTextMessage and AssistantToolCallMessage in the same file.
docs/test.ts pins all four cases, two valid and two behind
ts-expect-error. Those assertions are load-bearing: reverting the type to
all-optional fields fails type:doc:test with TS2578 twice.
The union is enforced on a directly annotated ImagePart but not on an
inline literal passed to annotate(), since inputData and outputData
include a "{ [key: string]: any }" arm that disables excess-property
checking. Narrowing that affects every annotate() shape and is left out.
Tests: the image non-string-content case now asserts the
invalid_io_messages telemetry tag that its audio counterpart already
asserted, closing a hole where deleting the tag argument kept the suite
green. An SDK-level image test mirrors the audio one, and three image
test names are aligned to the audio wording.
## Summary The file-limit boundary test spends most of its runtime opening and deleting 10,000 real files, which can exceed Mocha's 30-second timeout on Windows. ## Why The limit still needs the last accepted and first rejected case, so only the filesystem backend is replaced while all 10,001 production sink calls remain.
…ontext (#9575) A batch that mixes instrumented and uninstrumented messages attributed the ones without a context to the previous message's producer, so DSM reported edges no producer ever wrote. 1. `setDataStreamsContext` ignored a falsy context and left the previous message's pathway active; it now clears. 2. The SQS and Kinesis consumers skipped the decode for a message without a carrier, so nothing cleared the pathway. 3. `DsmPathwayCodec.decode` read the carrier through `pick` before its own null check and threw for a context-free message under `DD_TRACE_DEBUG`.
The native producer wrapper only forwarded the seventh-argument headers it recognized to the diagnostic-channel message, so trace injection replaced the caller's entire native header list instead of the fields propagation actually wrote, dropping application headers, ordering, repeats, and casing. 1. `Producer.produce()` now merges only the exact propagation fields into the caller's native header list, keeping every other entry, its order, and its repeats untouched. 2. KafkaJS maps and native consumers expose repeated wire headers differently (arrays vs. one-key records per repeat), so header conversion and DSM payload sizing now walk both shapes the same way and count wire records instead of array indices. 3. Repeated propagation fields (baggage, tracestate, DSM pathway context, …) now go through one field-owned read/write policy in `carrier.js` instead of raw carrier access per call site, so list fields combine, singleton fields resolve to the last usable value, and `traceparent` rejects repeats per the W3C Trace Context spec. An ESLint rule enforces that call sites use this policy instead of reaching into carriers directly. Refs: #9588 Refs: https://www.rfc-editor.org/rfc/rfc7230#section-3.2.2 Refs: https://www.w3.org/TR/trace-context/#tracestate-header-field-values
Add DD_TRACE_HTTP_SERVER_ERROR_STATUSES with DD_HTTP_SERVER_ERROR_STATUSES as its fallback alias and compile valid 100-599 ranges once in shared web configuration. Next.js now uses the same matcher as the other web plugins. Server spans hardcoded 5xx responses, so Node.js ignored the cross-tracer HTTP server error-status configuration. The existing validateStatus callback remains the programmatic override. - Run config and web utility unit tests. - Run the full HTTP server plugin test file. - Run the targeted Next.js 16 integration test. - Run changed-line coverage, generated config verification, and the full lint suite. Fixes: #7060
* feat(mysql,mysql2): trace pool connection acquisition An explicit pool.getConnection() held for a transaction hid any time spent waiting for a busy pool, and a pooled query never surfaced its acquire wait. Each explicit acquire now opens a dedicated acquire span (mysql.pool.acquire / mysql2.pool.acquire) carrying a pool.wait_time metric and recording connection errors; the acquire that pool.query() / execute() runs internally reports its wait as a tag on the query span instead, so a given acquire is counted once. Refs: #1613 * fix(mysql2): preserve pool-query acquire across cluster failover retries A pool cluster namespace retries `getConnection` on the next node when the first acquire fails, and with `canRetry` (the default) that retry is dispatched from the first acquire's asynchronous failure callback — after `wrapPoolQueryMethod` has already cleared the synchronous pool-query flag. The failover acquire was therefore treated as an explicit user acquire, opening a standalone `mysql2.pool.acquire` span and dropping the `pool.wait_time` tag from the successful query span. The namespace `getConnection` now re-asserts the flag for acquires that belong to a pool query, recognising retries by their reused callback. * fix(mysql): fold pool-cluster query acquire into the query span A `mysql` pool cluster's `PoolNamespace#query` acquires its connection internally, but that acquire was not bracketed with the pool-query flag, so it opened a standalone `mysql.pool.acquire` span and dropped the `pool.wait_time` tag from the query span — unlike the regular `pool.query` path. Bracketing `PoolNamespace#query` folds the wait into the query span; a `canRetry` failover retries by re-invoking `query`, so the same bracket also covers the node it fails over to. * ci: exercise the mysql instrumentation spec The new mysql instrumentation spec under packages/datadog-instrumentations/test only runs when a workflow sets PLUGINS=mysql for test:instrumentations; no job did, so verify-exercised-tests fails and the spec would never run in CI. The new job mirrors instrumentation-mysql2's service container and pinned image SHA. * fix(mysql,mysql2,pg): preserve pool acquire classification Pool cluster retries and connection callbacks can cross an async boundary, causing an internal query acquire to be reported as explicit and dropping its pool wait time. Stable query or callback identity preserves that classification. Synchronous implementations keep the existing fast path. The synchronous wait transfer measured 29.65-29.74 ns/op with WeakMap storage and 8.38-8.39 ns/op with the stack handoff on Node.js 24.18.0. * refactor(mysql): reduce pool acquire instrumentation churn ## Summary - fold pool query classification into the existing mysql and mysql2 wrappers - consolidate shared pool acquire control flow and equivalent contract tests - retain pg on the same synchronous fast path ## Why The implementation carried duplicate wrappers and test setup that obscured the hot-path invariants. This keeps subscriber-off forwarding and synchronous wait handoff allocation-free while preserving deferred dispatch and cluster retry isolation. ## Test plan - npm run lint - run the pool acquire helper, mysql, mysql2, and pg instrumentation suites - run the mysql and mysql2 plugin suites - verify changed-line and branch coverage against origin/master * fix(mysql,mysql2,pg): trace terminal pool acquisition failures Pooled queries suppress the explicit acquire lifecycle because their wait normally moves to the query span. A connection failure creates no query span, which dropped both the wait and error. Emit a backdated acquire lifecycle only for the terminal failure. Pool-cluster retries retain classification until the final callback, and synchronous mysql2 stream construction finishes the explicit acquire before rethrowing. * fix(mysql,mysql2): finish pool acquire spans through plugin lifecycle ## Summary Use the context-backed outbound lifecycle for explicit MySQL and MySQL2 pool-acquisition spans. ## Why Direct span completion bypassed peer-service computation, mapping, and serverless overrides. It also kept a second span lifecycle beside the PostgreSQL path. ## Drive-by Align pool-helper JSDoc and the MySQL instrumentation checkout action with current master. ## Test plan - PLUGINS=mysql|mysql2|pg npm run test:plugins - full changed-line coverage against origin/master - npm run lint
Bumps the testing-and-build group with 1 update in the /packages/dd-trace/test/plugins/versions directory: [mocha](https://github.com/mochajs/mocha). Updates `mocha` from 11.7.6 to 11.8.0 - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/v11.8.0/CHANGELOG.md) - [Commits](mochajs/mocha@v11.7.6...v11.8.0) --- updated-dependencies: - dependency-name: mocha dependency-version: 11.8.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: testing-and-build ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Overall package sizeSelf size: 7.97 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.3 | 125.43 kB | 441.68 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 9d90f8d | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-07 16:22:54 Comparing candidate commit 9d90f8d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2318 metrics, 40 unstable metrics.
|
4d7a229 to
9137ad3
Compare
Co-authored-by: gh-worker-campaigns-3e9aa4[bot] <244854796+gh-worker-campaigns-3e9aa4[bot]@users.noreply.github.com>
Bumps the test-versions group with 1 update in the /integration-tests/esbuild directory: [openai](https://github.com/openai/openai-node). Updates `openai` from 7.3.0 to 7.4.0 - [Release notes](https://github.com/openai/openai-node/releases) - [Changelog](https://github.com/openai/openai-node/blob/main/CHANGELOG.md) - [Commits](openai/openai-node@v7.3.0...v7.4.0) --- updated-dependencies: - dependency-name: openai dependency-version: 7.4.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: test-versions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… 2 updates (#9726) Bumps the testing-and-build group with 2 updates in the /packages/dd-trace/test/plugins/versions directory: [@electron/packager](https://github.com/electron/packager) and [next](https://github.com/vercel/next.js). Updates `@electron/packager` from 20.0.4 to 20.1.1 - [Release notes](https://github.com/electron/packager/releases) - [Changelog](https://github.com/electron/packager/blob/main/NEWS.md) - [Commits](electron/packager@v20.0.4...v20.1.1) Updates `next` from 16.2.12 to 16.3.0 - [Release notes](https://github.com/vercel/next.js/releases) - [Commits](vercel/next.js@v16.2.12...v16.3.0) --- updated-dependencies: - dependency-name: "@electron/packager" dependency-version: 20.1.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: testing-and-build - dependency-name: next dependency-version: 16.3.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: testing-and-build ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…pdates (#9728) Bumps the test-versions group with 6 updates in the /packages/dd-trace/test/plugins/versions directory: | Package | From | To | | --- | --- | --- | | [@anthropic-ai/claude-agent-sdk](https://github.com/anthropics/claude-agent-sdk-typescript) | `0.3.220` | `0.3.221` | | [@wdio/cli](https://github.com/webdriverio/webdriverio/tree/HEAD/packages/wdio-cli) | `9.30.0` | `9.30.1` | | [@wdio/jasmine-framework](https://github.com/webdriverio/webdriverio/tree/HEAD/packages/wdio-jasmine-framework) | `9.30.0` | `9.30.1` | | [@wdio/local-runner](https://github.com/webdriverio/webdriverio/tree/HEAD/packages/wdio-local-runner) | `9.30.0` | `9.30.1` | | [@wdio/mocha-framework](https://github.com/webdriverio/webdriverio/tree/HEAD/packages/wdio-mocha-framework) | `9.30.0` | `9.30.1` | | [pnpm](https://github.com/pnpm/pnpm/tree/HEAD/pnpm11/pnpm) | `11.19.0` | `11.20.0` | Updates `@anthropic-ai/claude-agent-sdk` from 0.3.220 to 0.3.221 - [Release notes](https://github.com/anthropics/claude-agent-sdk-typescript/releases) - [Changelog](https://github.com/anthropics/claude-agent-sdk-typescript/blob/main/CHANGELOG.md) - [Commits](anthropics/claude-agent-sdk-typescript@v0.3.220...v0.3.221) Updates `@wdio/cli` from 9.30.0 to 9.30.1 - [Release notes](https://github.com/webdriverio/webdriverio/releases) - [Changelog](https://github.com/webdriverio/webdriverio/blob/main/CHANGELOG.md) - [Commits](https://github.com/webdriverio/webdriverio/commits/v9.30.1/packages/wdio-cli) Updates `@wdio/jasmine-framework` from 9.30.0 to 9.30.1 - [Release notes](https://github.com/webdriverio/webdriverio/releases) - [Changelog](https://github.com/webdriverio/webdriverio/blob/main/CHANGELOG.md) - [Commits](https://github.com/webdriverio/webdriverio/commits/v9.30.1/packages/wdio-jasmine-framework) Updates `@wdio/local-runner` from 9.30.0 to 9.30.1 - [Release notes](https://github.com/webdriverio/webdriverio/releases) - [Changelog](https://github.com/webdriverio/webdriverio/blob/main/CHANGELOG.md) - [Commits](https://github.com/webdriverio/webdriverio/commits/v9.30.1/packages/wdio-local-runner) Updates `@wdio/mocha-framework` from 9.30.0 to 9.30.1 - [Release notes](https://github.com/webdriverio/webdriverio/releases) - [Changelog](https://github.com/webdriverio/webdriverio/blob/main/CHANGELOG.md) - [Commits](https://github.com/webdriverio/webdriverio/commits/v9.30.1/packages/wdio-mocha-framework) Updates `pnpm` from 11.19.0 to 11.20.0 - [Release notes](https://github.com/pnpm/pnpm/releases) - [Commits](https://github.com/pnpm/pnpm/commits/v11.20.0/pnpm11/pnpm) --- updated-dependencies: - dependency-name: "@anthropic-ai/claude-agent-sdk" dependency-version: 0.3.221 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: test-versions - dependency-name: "@wdio/cli" dependency-version: 9.30.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: test-versions - dependency-name: "@wdio/jasmine-framework" dependency-version: 9.30.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: test-versions - dependency-name: "@wdio/local-runner" dependency-version: 9.30.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: test-versions - dependency-name: "@wdio/mocha-framework" dependency-version: 9.30.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: test-versions - dependency-name: pnpm dependency-version: 11.20.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: test-versions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
9137ad3 to
e007da6
Compare
* feat(appsec): implement RFC-1103 normalized HTTP route tag for Express
Adds `_dd.appsec.normalized_route` span tag on every Express request when
API Security is enabled, converting framework-specific route syntax to the
RFC-1103 normalized form (e.g. `/api/:version/users/:id` → `/api/{version}/users/{id}`).
Supports Express 4 and 5, named/optional/catch-all params, multi-param segments
(`:a.:b` → `{a+b}`), and correctly resolves optional params for sub-routers
with `mergeParams=false` by matching against the request URL.
Performance: routes are compiled once per unique route string and cached;
non-optional routes hit a Map lookup on every request (~25 ns).
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* fix(appsec): use optional call for context().getTag in normalized route check
context().getTag is absent on mock spans used in unit tests; use ?.getTag?.()
to avoid a TypeError when the context object does not implement getTag.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* test(appsec): add coverage tests for normalized-route-express edge cases
Cover previously-uncovered code paths to satisfy the codecov/patch 95% threshold:
- trailing static text in getSegmentRegex and buildGenericSegmentRegex
- buildGenericSegmentRegex fallback (invalid constraint regex)
- named wildcard capture in matchSegs via URL extraction
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* refactor(appsec): rename normalized-route-express to normalized-route, add component dispatch
Renames api_security/normalized-route-express.js → normalized-route.js to
prepare for multi-framework support. Adds a normalizeRoute(component, ...)
dispatcher with a switch on the component tag (express now; other frameworks
to follow). The call site in appsec/index.js passes the component from the
span instead of gating on it.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* refactor(appsec): address PR review — pass req to normalizeRoute, move rootSpan inside guard
- normalizeRoute now takes req and extracts component/route/params/urlPath internally
(web module imported into normalized-route.js)
- rootSpan is now computed inside the if (route) guard in incomingHttpEndTranslator,
avoiding wasted work when route is empty
- Remove the '// Public API' section separator (flagged as not needed)
- Export normalizeRouteExpress for unit testing
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* refactor(appsec): lazy evaluation in normalizeRoute, simplify call site
- index.js: remove route pre-check, call normalizeRoute(req) directly;
web.root(req) only fetched when result is non-null (tag is to be set)
- normalizeRoute: check component first and return null early for
unsupported frameworks; route/urlPath extracted only for matched case
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* feat(appsec): support Express 5 {/:param} optional-group syntax in normalized route
Adds expandV5OptionalGroups() which converts Express 5 {/:id} optional-group
syntax to equivalent :id? form before processing, enabling full normalization
support for the standard Express 5 optional-segment pattern.
Supported conversions:
/items{/:id} → /items/:id? → /items/{id} or /items
/api{/:version}/users → /api/:version?/users
/photos/:id{.:format} → /photos/:id.:format?
/posts{/:id.:format} → /posts/:id?.:format?
Groups with only static content ({/draft}) are still rejected.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* feat(appsec): rewrite normalized-route around a route tokenizer
Replaces the expand+split core with a parse-once tokenizer that compiles a route
into segment templates, then renders + URL-matches from that model. This adds full
Express 5 support and resolves the open review threads:
- Optional static groups: /posts{/draft} → /posts/draft | /posts
- Optional catch-all groups: /files{/*path} → /files/{path} | /files
- Quoted param names: /users{/:"user-id"} → /users/{user-id}
- Nested optional groups: /a{/:b{/:c}} → /a/{b}/{c} | /a/{b} | /a
- Express 4 inline constraints incl. slash: /:id([^/]+) → /{id}
- Duplicate names: the last occurrence keeps the name, earlier (shadowed) ones
become paramN — /:id/:id → /{param1}/{id} (RFC rule 4 uniqueness)
Caching keys on the raw route string (parse/compile once); optional routes cache
rendered output per presence bitmask. Internals (parseRoute/compileRoute/renderRoute)
are exported for unit testing; the spec de-aliases the import and adds per-function
and dispatcher tests plus the full case matrix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): address review findings in normalized-route tokenizer
- Capture groups inside an inline constraint no longer corrupt optional-group
detection: presence markers are named captures (?<_ddgN>) read via m.groups,
immune to capture-index shifts (e.g. /:id(fo(o)).:format? → /{id}).
- Param + catch-all in one segment now combines names: /x/:a-* → /x/{a+param1}.
- Name uniqueness is enforced on ENCODED names so two raw names that encode
identically can't collide: /:"a/b"/:"a%2Fb" → /{param1}/{a%2Fb}.
- Backslash-escaped reserved chars are treated as static (Express 5):
/file/\{id\} → /file/%7Bid%7D.
- A non-terminal param whose constraint can consume '/' → null (rule 5);
terminal one is kept as the tail element.
- Guard pathological routes: cap optional groups at 24 (bitmask stays in 32 bits)
and bound the backtracking matcher with a step budget (24 optionals on a
non-matching URL: ~1500ms → ~1ms, falls back to req.params).
- Only treat a token as intra-segment-optional when its group is a strict
descendant of the segment group; harden m.groups read and the regex fallback.
- groupActive guards an undefined parent; add `variants` to the typedef.
Adds regression tests for each finding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): round-3 review fixes for normalized-route
- ReDoS: never embed a developer inline constraint in the URL matcher when it is
catastrophic (nested-quantifier heuristic), invalid, or contains a named group;
use a generic [^/]+? matcher instead. Constraint values are discarded from the
output anyway. (/:id((a+)+$)?/x on a long URL: ~1200ms → <1ms.) This also fixes
the named-marker collision when a constraint contains (?<...>).
- Step-budget abort now omits the tag (null) instead of guessing from req.params;
a clean URL/route mismatch still falls back to params.
- The `?` modifier only absorbs a true delimiter ('.') as its optional prefix, not
arbitrary preceding static: /x:id? on /x → /x (was /), /foo/v:id? on /foo/v → /foo/v.
- Non-terminal slash-consuming constraints: also reject a literal '/' in the source
and test more samples (/:id(foo/bar)/tail → null).
- Static optionals are matched in encoded form so non-ASCII matches the encoded URL
(/posts{/café} on /posts/caf%C3%A9 → /posts/caf%C3%A9).
- Param names: accept Unicode letters and $ ; handle escaped quotes in quoted names.
- Lower MAX_OPTIONAL_GROUPS to 12 (bounds the per-route variant cache to 4096).
Adds regression tests for each finding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): round-4 review fixes for normalized-route
Point 1 — eliminate the ReDoS class definitively: developer inline constraints are
NEVER embedded in the URL matcher (always a generic [^/]+? matcher), so a crafted URL
can never trigger catastrophic backtracking in a developer regex (e.g. a*a*a*…$ or
(a+)+). req.params is now the AUTHORITY for which optional params are present — it is
what Express populates — and the URL matcher is used only to resolve optionals absent
from req.params (mergeParams) or static/wildcard-only optional groups. This preserves
adjacent-optional disambiguation (/:a(\d+)?/:b? → /{b} when Express set b) without any
constraint execution on URL input.
Point 2 — static segments match case-insensitively (Express default routing); the
normalized output still preserves the route's declared case.
Point 3 — resolvePresenceFromUrl returns an explicit { present, aborted } instead of
relying on a module-global flag read after the fact.
Point 4 — thread the Express major version (instrumentation → apm:express:request:handle
→ tracing plugin → web.setFramework → web context → normalizeRoute). Express 4 routes
now parse with the v4 dialect: `{}` are literal characters (/file/{id} → /file/%7Bid%7D),
`*` is an unnamed wildcard, and bare `?`/`+`/`(` string-patterns return null. Express 5
(default when version unknown) keeps {…} groups, :"quoted" names, and *name wildcards.
The route cache key includes the dialect.
Point 5 — matcher edges: a catch-all segment now matches its non-wildcard prefix before
consuming the rest (/:id?/:a-* on /y-z/w → /{a+param1}); a terminal param whose
constraint can consume '/' is treated as a catch-all so mergeParams parent params are
recovered (/api/:version?/files/:rest(.+) → /api/{version}/files/{rest}).
Adds regression tests (incl. v4-dialect cases via the isV5 arg) for every finding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(appsec): address review comments — drop section-divider rules, add /a//b test
- Remove the // ---- divider rule lines (keep one-line section labels), per review.
- Add a regression test for empty-segment collapse (/a//b → /a/b, rule 2),
completing the requested v4/v5 case matrix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): round-5 review fixes for normalized-route
HIGH — the round-4 req.params-authoritative fast path was unsound: when req.params
held any optional param it skipped URL matching entirely, which (a) defeated the
mergeParams=false recovery this PR is built for (a dropped parent param was marked
absent → wrong lower-cardinality tag) and (b) lit up the wrong group when a param name
is shared across groups. Fix: the URL is authoritative again; req.params is used only to
BIAS the backtracking order (try the params-named branch first), so URL structure decides
what matches while ambiguous adjacent optionals still resolve to the param Express set.
When req.params names none of the route's optionals, the matcher keeps Express's greedy
present-first order. Removes the dead optionalParamNames/hasGroupNeedingUrl machinery.
MED — Express 4 dialect fidelity: parseName accepts only [A-Za-z0-9_] (no quoted/$/Unicode
names) and a `[` char-class string-pattern is rejected under isV5=false.
LOW — consumeParens skips escaped chars, so a constraint like :id(foo\)) is accepted.
LOW — the wildcard-prefix regex is cached per segment instead of rebuilt per request.
Verified: mergeParams recovery, shared-name, greedy-empty-params, and the constraint
disambiguation cases all correct; ReDoS still ~0.1ms. Adds a regression test per finding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): round-6 review fixes for normalized-route
- #3 (regression from round 5): constraintMatchesSlash no longer treats a '/' that
appears only inside a character class as slash-spanning. `[^/]+` denies slashes, so
/:id([^/]+)/users (Express 4) normalizes to /{id}/users again instead of null.
- #2: a non-terminal catch-all (a wildcard with a non-empty segment after it, incl. an
optional {/*rest}/tail) is rejected at compile (→ null) rather than silently dropped.
- #1: a segment containing two independent optional groups (e.g. :a{.:b}{-:c}) is
rejected (→ null); our single per-segment regex can't replicate path-to-regexp's
ordered-alternative assignment, so the combined name could be wrong. Single
intra-segment optionals (:id{.:format}) are unaffected.
- #4: structural-only optional groups — a {...} wrapping only nested group(s) with no
segment/token of its own (e.g. the outer braces in /a{{/b}}) — are collapsed by
reparenting represented groups to their nearest represented ancestor, so /a{{/b}}
resolves to /a/b | /a instead of always /a.
Adds a regression test per finding (verified against live Express 5).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): normalize Express routes via path-to-regexp parse()
Rebuild the RFC-1103 normalized-route computation on top of path-to-regexp
v8's own parse() token tree instead of a hand-written route tokenizer.
Reusing the framework parser removes ~250 lines of grammar code and the
whole class of corner-case parsing bugs, and keeps us from drifting away
from path-to-regexp's semantics.
This is Express 5 only: path-to-regexp v8 is the parser Express 5 ships and
the one that exposes parse(). Express 4 ships path-to-regexp 0.x, which has
no parse(); getParse() returns undefined there and we omit the tag. Express
4 route syntax (:id?, :id(regexp), unnamed *, :name+/:name*) is rejected by
the v8 parser anyway, so a real Express 5 app cannot register it.
- path-to-regexp instrumentation: expose getParse() (captures the v8 token
tree adapter), mirroring the existing getCompileToRegexp().
- normalized-route: add tokensToSegments() adapter over parse().tokens;
keep the proven render / URL-presence / backtracking-matcher logic and
the terminal-catch-all and structural-only-group guards. Drop the custom
parser, inline-constraint handling and v4/v5 dialect threading.
- Newly supported: multiple independent optional groups in one URL segment
(e.g. /:a{.:b}{-:c}) now combine correctly into one atomic element.
- appsec/index: drop misleading inner optional chaining on the guaranteed
apiSecurity.enabled boolean.
- Tests: unit spec exercises the normalizer against the real v8 parser;
integration spec asserts the tag is present on Express 5 and absent on
Express 4.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): drop dead code left by the parse()-based rewrite
The parse()-based normalizer decides the Express dialect from getParse()
availability, so the framework-version plumbing added in earlier rounds is
no longer read by anything. Revert it to keep production changes minimal:
- express.js, express plugin tracing.js, web.js: restore to master (the
expressMajor capture, the handle-channel payload field, and the
setFramework frameworkVersion parameter had no remaining consumer).
Also trim dead code inside normalized-route.js:
- Stop exporting renderRoute / resolvePresenceFromUrl (no test imports them;
keep the public surface minimal).
- Remove the wildcard `zeroOrMore` field and its unreachable guard — v8's
parse() never yields a required (`+`) catch-all, so it was always true.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(appsec): drop redundant comments in normalized-route
Remove section-divider comments that only restated the JSDoc of the
function directly below them, a cache comment that restated its variable
name, and an inline comment duplicating its function's JSDoc. Tighten the
path-to-regexp getParse capture comment. Keeps only comments that carry
non-obvious intent the code can't.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): drop req.params biasing from the route matcher
The present/absent ordering bias existed to disambiguate adjacent optional
params via their inline regex constraints — an Express 4 feature the
parse()-based normalizer no longer supports (path-to-regexp v8 has no inline
constraints). v8 matches greedily left-to-right, so plain greedy-present-first
backtracking already resolves presence exactly as Express did.
Removes optionalParamInParams, segParamInParams, the matchParamsInformative
module state, and the params argument threaded through matchSegments /
matchSegmentHere / resolvePresenceFromUrl. No behavior change (95 unit tests
unchanged); ~70 fewer lines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): guard apiSecurity access in normalized-route gate
config.appsec.apiSecurity can be undefined for partially-built configs, so
`config?.appsec.apiSecurity.enabled` threw "Cannot read properties of
undefined (reading 'enabled')" in the HTTP-end translator — an uncaught throw
on every request path when appsec is enabled, breaking web-framework and
instrumentation suites broadly. Restore full optional chaining.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): gate normalized route on the real API Security config flag
The gate read `config.appsec.apiSecurity.enabled`, but the config exposes the
flag as `config.appsec.DD_API_SECURITY_ENABLED` (the property the API Security
sampler itself reads). `apiSecurity` is never a nested object on the config, so
the old path was always undefined: the non-optional form threw on every request
(broad CI breakage) and the optional form silently never set the tag. Use the
canonical DD_API_SECURITY_ENABLED flag so the tag is emitted when API Security
is on and omitted when it is off.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): cover wildcard-prefix path and drop two unreachable branches
codecov/patch was 94.32% (target 95%). The gap was the static-prefixed
wildcard path, which no unit or integration test exercised. Add unit cases for
`/files{/:opt}/v*rest` that drive getWildcardPrefixRegex and both outcomes of
the prefix check (including a backtrack past a prefix mismatch).
While confirming coverage, two branches turned out to be unreachable given the
code's own contracts, so remove them rather than test dead code:
- compileRoute's try/catch around parse(): the getParse adapter already
swallows parser throws and returns undefined, so parse() never throws here.
- getSegmentMatcher's wildcard branch: matchSegmentHere routes wildcard
segments to the catch-all branch before ever calling getSegmentMatcher, so
it only sees static/param tokens.
The remaining uncovered lines are two deliberate hot-path crash guards
(surrogate-encode fallback, RegExp-construction fallback).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): drop six behaviorally-duplicate normalized-route cases
A per-test coverage attribution showed these six each share an identical
statement+branch footprint with a sibling that already exercises the same
behavior and route shape, so removing them leaves line/branch/statement
coverage unchanged (307/326 stmts, 178/205 branches, 254/265 lines):
- "works when params is undefined" (== the params-is-null case)
- "combines two params separated by a dash" (== the ':id.:format' combine)
- "normalizes /app/*splat with mount prefix" (== '/files/*rest')
- "still normalizes a terminal named wildcard" (== '/files/*rest')
- "handles deeply nested mount paths" (== 'includes mount prefix')
- "req.params only biases ordering..." (exact dup of the greedy first-wins
case; its premise is stale since the params biasing was removed)
Behaviorally-distinct permutations (delimiters, char classes, present/absent
branches, independent optional groups, rejected v4 syntaxes) are kept — those
guard regressions coverage numbers can't see.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): reject un-representable route segments; harden + tune normalizer
Addresses the multi-agent review of the normalized-route feature.
Correctness:
- Reject single-segment shapes the delimiter-agnostic per-segment matcher would
mis-assign, rather than emit a wrong tag: a non-terminal wildcard within a
segment (`/files/*path.:ext`, `/*a-*b`) and more than one intra-segment
optional group (`/:a{.:b}{-:c}`, nested `/:a{.:b{.:c}-:d}`). These previously
produced incorrect combinations (e.g. `/:a{.:b}{-:c}` on `/x-y.z` → `/{a+c}`
instead of path-to-regexp's `/{a+b}`). Now they return null.
- resolvePresenceFromParams counts a present-but-empty param value as present
(`!== undefined` instead of truthiness).
- Document that a root request arrives as route '' → null, intentionally
matching http.route (also omitted for the empty route).
Perf (optional-route matcher hot path only; common precomputed route unchanged
at ~4ns/call):
- Precompute a per-segment wildcardIndex instead of rescanning tokens twice per
matcher step; drop the now-unused segmentWildcard helper.
- Store prebuilt marker-name strings in the presence list (no per-read rebuild).
- Skip the rollback-array allocation for segments with no intra-optional groups.
- Split the URL path in one pass instead of split().filter(Boolean).
Minimalism:
- Drop the tokensToSegments/compileRoute test-only exports and their
implementation-detail tests; keep normalizeRouteExpress as the single core
seam (the dispatcher is covered by the express integration spec).
- Note the process-global parse adapter and route-cache growth bounds.
Unit coverage 96% lines; behavior verified against path-to-regexp v8's match().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): reject param/wildcard intra-segment optional groups; drop dead guards
Second review round follow-up.
Correctness (reject rather than mis-normalize):
- A param or wildcard inside an intra-segment optional group is now rejected
(returns null). Our delimiter-agnostic '[^/]+?' matcher diverged from
path-to-regexp for these: '/photos/:id{.:format}' on '/photos/1..' assigns
{ id: '1..' } (format absent) but we emitted '/{id+format}'; and an optional
group before a same-segment wildcard ('/:a{.:b}-*rest') dropped the optional's
presence because the wildcard branch bypasses the segment matcher. Static-only
intra-segment optional groups (e.g. '/foo{bar}') stay supported. Whole-segment
optionals ('/items{/:id}', '/posts{/:id.:format}', nested '/a{/:b{/:c}}') and
mandatory multi-param segments are unaffected.
Dead code:
- Remove renderRoute's precomputed short-circuit (unreachable: both callers pass
precomputed===null) and its 'present segment after a catch-all' bail
(unreachable given the compile-time non-terminal-wildcard guard); renderRoute
now never returns null (return type + variants map tightened accordingly).
- Fix an orphaned JSDoc block: splitPathSegments had been inserted between
normalizeRouteExpress's doc comment and its definition.
Verified against path-to-regexp v8 match(): rejected shapes → null, all retained
shapes match. 85 unit + 101 appsec-index tests pass; 96% unit line coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): fix wildcard/multi-segment presence bugs; match literal; trim
Final review-round follow-up (differential fuzz vs path-to-regexp v8 match()).
Correctness:
- A required terminal wildcard no longer matches zero URL segments, which had
let a preceding optional be marked present. '/files{/:id}/*rest' on
'/files/x' now yields '/files/{rest}' (was '/files/{id}/{rest}').
- Reject an optional group that directly spans >1 URL segment ('{/:a/:b}'):
it is atomic in path-to-regexp but our matcher toggled its segments
independently, so a sibling optional could steal one ('{/:a}{/:b/:c}' on
'/B/C' gave '/{a}'; now null).
- Match static segments against the LITERAL route text (what Express/
path-to-regexp match against the raw URL) instead of the re-encoded form;
the encoded form is only for rendering. Fixes double-encoding ('/x{/a%40b}')
and literal non-ASCII statics.
- Reject optional trailing/interior slash groups ('/users{/}', '/items{/:id/}')
and adjacent dynamic tokens with no static between ('/:a:b', '/:a*rest',
which Express itself rejects at registration).
Efficiency:
- Strip the URL query string lazily inside normalizeRouteExpress, past the
precomputed early-return (no slice on the common cached path).
- Resolve the request context once in normalizeRoute (was web.root + getContext).
- Store the per-segment matcher/prefix regex on the segment object instead of
two module-level Maps.
Dead code:
- Remove getSegmentMatcher's unreachable RegExp-construction try/catch (pattern
can't throw; the appsec hook already wraps the call) and groupActive's
unreachable `g === undefined` guard.
Verified against path-to-regexp v8 match(); 89 unit + 101 appsec-index tests
pass; 98% unit line coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): reject multi-segment/prefixed-wildcard optional shapes; reuse http.route; trim
Follow-up to the latest multi-agent review (differential fuzz vs path-to-regexp v8).
Correctness (reject rather than mis-normalize):
- Reject an optional group that spans more than one URL segment, counting a
group as present in a segment when it is the segment's group OR any token's
group. This now also catches a group that owns a token in one segment and a
full later segment ('/a{b/:c}{/:d}' on '/a/d' gave '/ab/{c}'; now null).
- Reject a wildcard that has a prefix in its segment ('v*rest', 'p{q}*rest')
once the route also has optional groups: the backtracking matcher then runs
and the wildcard-prefix regex can't resolve presence consistently with
path-to-regexp ('/files{/:opt}/v*rest', '/a{/:id}/p{q}*rest' → null). Without
optional groups the route is precomputed and a prefixed wildcard is fine.
This makes getWildcardPrefixRegex dead, so it and the matcher's prefix branch
are removed.
Efficiency:
- normalizeRoute now reuses the http.route tag (set by setRouteOrEndpointTag
just before this hook) instead of re-deriving the route from context.paths,
and resolves the span with a single web.root() lookup (was web.root +
web.getContext). Removes a per-request join allocation for nested routers and
a duplicate reconstruction of the route rule.
Dead code:
- Unnamed params/wildcards are impossible in path-to-regexp v8, so drop the
null-name handling (typedef, the `?? null`, the `!= null` guards in renderRoute
pass 2) and refresh the stale comment.
- Drop renderRoute's always-true pass-1 per-token group-active check (a dynamic
token can't sit in a deeper group than its segment).
Doc-only:
- Note that interior '//' URL segments are collapsed (a malformed-URL edge) and
keep the process-global getParse note.
Verified against path-to-regexp v8 match(); 89 unit + 101 appsec-index tests
pass; 98% unit line coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): keep v8 parser on re-hook; support multiple static optional groups
Addresses two Codex review comments.
- path-to-regexp: probe `parse()` at hook time and adopt it only if it returns
the v8 TokenData shape ({ tokens: [...] }). Previously, a later-loaded older
major (6.x/7.x `parse()` returns a bare array) re-ran the hook and overwrote
the working v8 adapter with one that always returns undefined, silently
disabling normalization (and caching null) for the rest of the process.
- normalized-route: allow any number of *static* intra-segment optional groups
('/a{b}{c}'). They are literal, so the named-marker segment matcher resolves
their presence exactly (verified against path-to-regexp match() over all
presence combinations). Only param/wildcard intra-segment optional groups —
which need delimiter-aware matching we can't replicate — remain rejected.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): drop optional chaining on the API Security gate
config is guaranteed non-null when incomingHttpEndTranslator runs (enable()
sets it before any event-loop turn; disable() nulls it and unsubscribes the
handler in the same synchronous call), and config.appsec.DD_API_SECURITY_ENABLED
is always a boolean. Address review feedback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): resolve optional-group presence via path-to-regexp match
Replace the custom backtracking route matcher with path-to-regexp v8's own
match() to resolve which optional groups a request filled. This reuses the
framework's matching instead of re-implementing it (per review feedback) and
cuts normalized-route.js from 721 to 448 lines.
Behavior changes, both toward "omit rather than mis-normalize":
- Static-only optional groups (/posts{/draft}, /a{b}{c}) have no capture key,
so match() cannot report their presence -> the route is omitted.
- An optional group sharing a param name with another token collapses to one
key in match()'s output -> presence is ambiguous, so the route is omitted.
- Intra-segment and multi-segment optional param groups that the old matcher
rejected (/photos/:id{.:format}, /x{/:a/:b}, {/:a}{/:b/:c}) are now resolved
correctly.
getMatch() is added to the path-to-regexp instrumentation as a version-probed
v8 match() factory, mirroring getParse().
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): table-drive normalized-route spec (495 → 249 lines)
Replace the one-assertion-per-it boilerplate with a check(route, url, expected,
params) helper that registers one test per case, named after its inputs so a
failure still names the exact route. Same coverage (95%/89%), 105 cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(appsec): trim comments in normalized-route
Shorten multi-line inline comments to their non-obvious core and drop pure
narration; keep the RFC-rules module doc and JSDoc. No behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): drop static-only optional groups instead of omitting the route
A static-only optional group (/posts{/draft}, /a{b}{c}, a bare optional slash
/users{/}) carries no param for match() to resolve. Rather than omit the whole
route, render it as absent — a stable, minimal normalized route. This also
rescues mixed routes: /a{/:id}/p{q}*rest now yields /a/{id}/{rest} (static
group dropped, param resolved) instead of null. Only a param group whose
presence is genuinely ambiguous (a shadowed name) still omits the route.
Also lower MAX_OPTIONAL_GROUPS from 12 to 8: path-to-regexp's match() builds a
regexp exponential in the optional-group count (first-call ~3s at 11 groups),
which timed out the many-optionals guard test on CI. At 8 the route is omitted
before a matcher is ever built.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): drop 5 coverage-redundant normalized-route cases
Remove cases that exercise a branch already covered by another: undefined/42
inputs (same non-string guard as null), two ASCII-encode statics (covered by
the dedicated encoding describe), and :path+ (same :name-modifier reject as
:path*). Coverage unchanged (95.7%/89.4%, identical uncovered lines).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): keep mandatory static that shares a segment with an optional group
Two correctness fixes found in review:
1. A group-0 (mandatory) static token can share a URL segment with an optional
group when no top-level slash separates them (e.g. '.json' in
'/files{/:id}.json', or the 'y' in '/x{/a}{/b}y'). renderRoute gated each
output element on the segment's leading-slash group, so when that group was
absent the whole segment — including the mandatory static — was dropped
('/files.json' rendered '/files'). renderRoute now flushes an element only at
a present leading slash and merges an absent-slash segment's still-present
tokens into the current element.
2. A group whose presence is detectable only via a param in a NESTED optional
subgroup (e.g. '/a{/b{/:c}}') mis-rendered '/a/b' as '/a'. Detectability now
requires a unique param the group holds directly, so such routes are omitted
rather than mis-normalized.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): cap total optional groups and tie match adapter to v8 parse
Two issues surfaced by a Codex review round:
1. The exponential-regexp guard counted only resolvable (detectable) groups,
but path-to-regexp's match() regexp is exponential in the route's TOTAL
optional-group count. A route like /r{a}...{t}{/:id} (20 static + 1 param)
passed the cap (1 detectable) yet took ~4s to build. Cap on the total group
count (groupParent.size) before building the matcher instead. Static-only
routes with no resolvable group still precompute cheaply (no matcher).
2. The path-to-regexp match() probe (`probe?.params`) can't distinguish v8 from
v6/v7 (all share the { params } shape), so a later-loaded older major could
clobber the v8 matcher. Capture parse() and match() together, gated on the
same v8 TokenData probe, so only a confirmed-v8 module installs either.
Also init the variants Map when building the matcher entry (drop the per-request
lazy check; precomputed entries never reach it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* perf(appsec): skip param decoding and gate normalized route on the v8 dialect
Review feedback from BridgeAR:
- match(route, { decode: false }): presence resolution reads which params
matched, never their values, so per-param decodeURIComponent is waste. ~55%
faster per match() call (218ns -> 99ns; 320ns -> 194ns per request). It also
stops a malformed escape ('/x/%ZZ') from throwing URIError inside the matcher,
which read as "no match" and silently degraded presence resolution to the
req.params fallback on attacker-controlled input.
- Expose the Express route grammar and require it to be v8. Previously a loaded
v8 path-to-regexp stood in for "this is Express 5", which is unsound twice
over: an Express 4 app can pull v8 in through an unrelated dependency
(path-to-regexp is hooked for any requirer), and a process can serve both
majors (see express-multi-version.spec.js). The grammars disagree — '/a{2}' is
a regex quantifier matching '/aa' in v4 but an optional group in v8 — so v4
routes were tagged '/a'. getExpressRouteDialect() now reports
'v8' | 'legacy' | 'mixed' | undefined and only 'v8' is normalized, so a v4 or
mixed process emits no tag rather than a wrong one.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): only let path-to-regexp 8 install the parse/match adapters
IlyasShabi spotted that 7.x also returns TokenData, so the `Array.isArray(
probe.tokens)` shape probe accepted it. Its tokens are bare strings rather than
typed nodes, so tokensToSegments matched no branch, produced no segments, and
every route normalized to '/' — a wrong tag on every request in any Express 5
app that also had path-to-regexp 7 in its tree.
Gate the capture on `versions: ['>=8']` instead, per their suggestion. Version
matching is the loader's job, so the probe and its try/catch are gone. Added a
regression spec that drives the registered hooks through the loader's own
semifies matching and asserts 7.x cannot install or replace the adapters
(verified failing against the previous ['*'] registration).
Also rename the route-dialect values to 'express5' | 'express4' | 'mixed' and
drop "v8" from prose: it read as the V8 JS engine rather than path-to-regexp 8.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* revert(appsec): drop the process-global Express-5 route dialect gate
The gate refused to normalize in any process where both Express majors were
loaded. That is sound in principle, but a process-global flag is set by any
express require anywhere in the process — including mocha's collection phase —
so in a shared test process every Express 5 request saw 'mixed' and lost the
tag. AppSec/express was green at d81decb and failed from e52d102 for exactly
this reason.
Reverting to the parser-availability check restores that behaviour and leaves
the known gap documented in place: an Express 4 app that pulls path-to-regexp 8
in through a dependency is read with Express 5 grammar. Closing it properly
needs the dialect of the router that recorded the route, resolved per request
(datadog-plugin-router's web.setRoute call site knows it), not a global flag.
Keeps the two independent fixes: the >=8 hook gating and match({decode:false}).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): classify express major by installed version, not the range
withVersions can hand the spec a range ('>=4') that intersects both majors
while the folder actually installs express 5, so semver.intersects(version,
'<5.0.0') reported express 4 and the app registered the wrong route syntax —
the server then 404'd on '/tree/main'. Resolve the installed version with
.version() and classify with satisfies(), as the next/mysql2 specs do.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ci: exercise the path-to-regexp instrumentation spec
verify-exercised-tests failed because no workflow glob reached the new
packages/datadog-instrumentations/test/path-to-regexp.spec.js. Add it to the
router job, whose dependency it is, rather than spend a runner on two tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): move the path-to-regexp spec under the service-free misc glob
The previous placement needed a PLUGINS entry to be exercised, and adding one
made install_plugin_modules demand a version range ("Latest version for
'path-to-regexp' needs to be defined in versions/package.json"), provisioning
module versions this spec never loads — it fabricates 7.x/8.x shaped modules.
test:instrumentations:misc globs test/*/**/*.spec.js and runs without
yarn services, which is what a pure unit test wants. Reverts the workflow edit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): mirror decode:false in the spec matcher, drop "v8" wording
Review feedback from IlyasShabi: the spec's makeMatcher mirror had drifted from
the instrumentation adapter, which now passes { decode: false }, and "v8" reads
as the V8 engine rather than path-to-regexp 8.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(appsec): pass client:false to the http plugin, not express
`client` is an http-plugin option (datadog-plugin-http/src/index.js:29), so in
the positional config array it was landing on express while http got {} —
client spans were never actually disabled. Spotted by IlyasShabi.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(appsec): decide the Express major per request, not per process
AppSec/express went red once the client:false fix landed. That fix was correct:
the http client spans it had been leaving enabled were satisfying the Express 4
"must not set the tag" assertion on their own, so it had been passing vacuously.
With the mask gone, the real defect showed: the parse/match adapters are
process-wide, so an Express 4 block running after an Express 5 one gets its
routes read with Express 5 grammar. In CI the >=4 folder resolves to 5.2.1 and
runs third, so the 4.2.0 and 4.3.0 blocks after it were tagged.
Decide the major per request from the serving app instead, via the legacy
app.del alias Express 5 removed. Verified against every provisioned version
(4.0.0-4.22.2 keep it, 5.0.0/5.2.1 do not). Being per-request, this also covers
a v4 sub-app mounted inside a v5 process, which no process-wide flag can.
The full fix remains the router's own dialect at the web.setRoute call site,
resolved per request; this is the small version of it, kept local to appsec.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
## Summary Use the existing 5s LocalStack window for DynamoDB trace assertions. ## Why LocalStack may respond after the mock agent's 1s default expires, which removes the observer before the SDK callback completes.
* fix(openai-agents): preserve structural span ancestry - OpenAI Agents 0.14 adds untraced task and turn spans to the default hierarchy. Track those parent links so traced descendants resolve to the nearest Datadog span and errored workflows finish correctly.
* feat(llmobs): resolve and propagate agent attribution
Every LLMObs span that has an agent ancestor now carries
meta.agent_attribution = { parent_agent_name, parent_agent_span_id }
identifying its nearest agent ancestor. The nearest agent is resolved
once at span registration (a one-level lookup that inherits the parent's
already-resolved attribution, no ancestor walk) and propagated across
service boundaries via the _dd.p.llmobs_parent_agent_id /
_dd.p.llmobs_parent_agent_name distributed tags. Spans with no agent
ancestor omit the block entirely.
The agent id is always digit-safe; an agent name that is not tagset-safe
(comma or non-printable byte, or over the byte budget) is skipped on the
wire so it cannot poison x-datadog-tags, and the backend resolves the
name from the id in that case.
This mirrors the dd-trace-py implementation (DataDog/dd-trace-py#18788)
and emits the identical wire field the backend pass-through already
carries (ddoghq/dd-source#5177, DataDog/dd-go#244115).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(llmobs): guard agent name injection against x-datadog-tags budget overflow
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore(llmobs): shorten agent attribution tag names to pagent_name/pagent_span_id
Rename wire-format and payload tag names to match dd-trace-py:
- _dd.p.llmobs_parent_agent_id → _dd.p.llmobs_pagent_span_id
- _dd.p.llmobs_parent_agent_name → _dd.p.llmobs_pagent_name
- parent_agent_name → pagent_name (meta.agent_attribution payload)
- parent_agent_span_id → pagent_span_id
Internal _ml_obs.* span meta keys are not renamed (tracer-internal only).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(llmobs): address review comments on agent attribution
- remove block comment before PARENT_AGENT_* constants in tags.js
- extract appendOptionalPropagatedTag utility in util.js
- move resolveAgentAttribution to util.js as a free function (tags, span)
- add TODO for span-kind mutation limitation in #tagAgentAttribution
- remove DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH budget guard from agent name
injection (same known limitation as ml_app, tracked separately)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(lint): fix line length and jsdoc type in agent attribution
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(llmobs): restore budget check and wipe stale pagent on injection
- reuse already-stored mlObsSpanTags instead of a redundant tagMap lookup
- restore DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH budget check in
appendOptionalPropagatedTag (dropped in the previous refactor)
- add stripTagsetEntry helper to remove stale upstream pagent_name /
pagent_span_id that _injectTags may have already written into the carrier;
when a local agent is resolved, both entries are stripped and re-injected
so the downstream sees a consistent id-only or id+name pair (product
decision: keep the id, wipe the name when unsafe)
- unit tests for appendOptionalPropagatedTag budget boundary and
stripTagsetEntry; integration tests for the stale-entry wipe cases
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(llmobs): gate agent name on id fitting within budget
When the x-datadog-tags budget fits the name but not the id, the previous
code would propagate a name without a span id — unresolvable by the
backend. Now the name is only appended after confirming the id was added;
both are dropped together when the id does not fit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* ci: re-trigger CI
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
e007da6 to
9d90f8d
Compare
There was a problem hiding this comment.
More details
The runtime changes were exercised across propagation edge cases, Express route normalization, LLMObs attribution and image parts, pool acquisition, HTTP status configuration, and Vitest/WebdriverIO retry flows. No diff-only behavioral regression or production-impacting hazard was reproduced; the few blocked runs were environment fixture/configuration issues and passed after isolating ambient variables.
📊 Validated against 20 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 9d90f8d · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Features
Fixes
Internal (CI, Testing, Benchmarking)